DOC: Document existing functionality of pandas.DataFrame.to_sql() #11886 #26795

oguzhanogreden · 2019-06-11T20:36:35Z

Adds documentation for ENH/SQL: provide single value to 'dtype' argument in to_sql as dtype for all columns #11886
doc build fails (may be related to DOC/CI: Doc build is failing with sphinx 2.1 #26723)
passes git diff upstream/master -u -- "*.py" | flake8 --diff
What's new? Adds documentation to an existing functionality. In fact, the functionality is documented at pandas.io.sql.to_sql but it's not visible here.

oguzhanogreden · 2019-06-11T20:41:19Z

The documentation related notes on "Contribute to Pandas" are not in a very helpful state. I haven't yet managed to build and check if this appears at pandas.DataFrame.to_sql(). I suppose it would, so I went on with the PR.

Next one in line is documentation contribution notes :)

WillAyd

Minor doc nits. Any interest in adding an annotation while you are looking at this?

pandas/core/generic.py

codecov · 2019-06-11T21:13:00Z

Codecov Report

❗ No coverage uploaded for pull request base (master@bc65fe6). Click here to learn what that means.
The diff coverage is 100%.

@@            Coverage Diff            @@
##             master   #26795   +/-   ##
=========================================
  Coverage          ?   91.87%           
=========================================
  Files             ?      179           
  Lines             ?    50698           
  Branches          ?        0           
=========================================
  Hits              ?    46578           
  Misses            ?     4120           
  Partials          ?        0

Flag	Coverage Δ
#multiple	`90.46% <100%> (?)`
#single	`41.1% <100%> (?)`

Impacted Files	Coverage Δ
pandas/core/generic.py	`94.2% <100%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bc65fe6...efa97be. Read the comment docs.

oguzhanogreden · 2019-06-12T06:33:10Z

I'll add the annotations.

WillAyd · 2019-06-12T13:10:28Z

I'll add the annotations.

Cool thanks. If it ends up being a lot of trouble we could go forward with just this for now and address annotations in separate PRs. lmk

Co-Authored-By: William Ayd <[email protected]>

pep8speaks · 2019-06-13T20:06:45Z

Hello @oguzhanogreden! Thanks for updating this PR. We checked the lines you've touched for PEP 8 issues, and found:

There are currently no PEP 8 issues detected in this Pull Request. Cheers! 🍻

Comment last updated at 2019-08-29 10:01:59 UTC

pandas/core/generic.py

oguzhanogreden · 2019-06-13T20:19:40Z

pandas/core/generic.py

+               index: bool = True,
+               index_label: Optional[Union[str, List[str]]] = None,
+               chunksize: Optional[int] = None, dtype: Union[dict] = None,
+               method: Union[str, Callable] = None):


Please see the Callable annotation here. It may be possible to be more precise here. I looked around a bit, finally decided not to write the most complex type annotation in the code base 🤷‍♂

You should be able to match this up with exact types (see the documentation for method)

pandas/io/sql.py

WillAyd

nice job!

pandas/core/generic.py

WillAyd · 2019-06-13T22:28:28Z

pandas/core/generic.py

+               schema: Optional[str] = None, if_exists: str = 'fail',
+               index: bool = True,
+               index_label: Optional[Union[str, List[str]]] = None,
+               chunksize: Optional[int] = None, dtype: Union[dict] = None,


For dtype use Dict[Any, Dtype] (Dict from typing, Dtype from pandas._typing)

WillAyd · 2019-06-13T22:29:40Z

pandas/core/generic.py

+               index: bool = True,
+               index_label: Optional[Union[str, List[str]]] = None,
+               chunksize: Optional[int] = None, dtype: Union[dict] = None,
+               method: Union[str, Callable] = None):


You should be able to match this up with exact types (see the documentation for method)

WillAyd

Thanks for the thorough investigation and input on this so far

pandas/core/generic.py

oguzhanogreden · 2019-06-15T12:56:07Z

Oops... circular import. I don't feel comfortable with the code base enough to work around this. I'll replace the annotation for pd_table with Any unless you have a suggestion.

WillAyd · 2019-06-15T13:00:12Z

OK sure. Any is not very useful as an annotation but can come back to that piece later; I think the rest of this is great

WillAyd · 2019-06-15T13:01:18Z

Just put a comment like TODO: replace Any with SQLTable if circular import resolved

oguzhanogreden · 2019-06-15T13:23:00Z

Sure, thanks for the way out here and patient help!

For completeness...

method: Union[str, Callable[[Any, Any, ...

can be replaced with

method: Union[str, Callable[[pandas.io.sql.SQLTable, Union[sqlalchemy.engine.Connection, sqlalchemy.engine.Engine], ...

This can be done when:

SQLTable can be imported without causing circular import,
sqlalchemy types are available for type annotations.

WillAyd · 2019-06-15T13:26:34Z

pandas/core/generic.py

+    # TODO: Replace `Callable[[Any, Any, ...` when SQLTable and sqlalchemy
+    # can be imported. SQLTable can't be imported due to circular import.
+    # sqlalchemy can't be imported since it's an optional dependency.
+    def to_sql(self, name: str, con,


Sorry should have asked this before but can you put each parameter on a separate line? Will help with readability

Done. Also added Any to con= and added explanation to the note so that less thinking is required later.

WillAyd · 2019-06-15T13:30:56Z

pandas/core/generic.py

@@ -50,6 +51,9 @@
 from pandas.io.formats.printing import pprint_thing
 from pandas.tseries.frequencies import to_offset

+# mypy confuses the `bool()`` method of NDFrame
+_bool = bool


Yea this is unfortunate and something we've seen before:

#26029 (comment)

The alias is the suggested approach so no change required here I think, but cc @jreback for visibility

WillAyd

Lgtm @jreback

pandas/core/generic.py

oguzhanogreden · 2019-06-17T06:10:52Z

I expected that VSCode would let me know of updates in master. The last conflict was due to that and I missed it before hitting resolve!

pandas/core/generic.py

WillAyd · 2019-06-17T16:01:46Z

Makes sense as sqlite3 is part of the stdlib whereas sqlalchemy is a third party package and I don’t think on typeshed.

…

Sent from my iPhone

On Jun 17, 2019, at 11:56 AM, Oğuzhan Öğreden ***@***.***> wrote: @oguzhanogreden commented on this pull request. In pandas/core/generic.py: > @@ -2458,8 +2462,22 @@ def to_msgpack(self, path_or_buf=None, encoding='utf-8', **kwargs): return packers.to_msgpack(path_or_buf, self, encoding=encoding, **kwargs) - def to_sql(self, name, con, schema=None, if_exists='fail', index=True, - index_label=None, chunksize=None, dtype=None, method=None): + # TODO: Replace `con: Any` and `Callable[[Any, Any, ...` when SQLTable + # and sqlalchemy can be imported. SQLTable can't be imported due to I got the correct type for sqlite3 when I tried reveal_type(). @WillAyd did you notice the if TYPE_CHECKING: imports? It seems I'm making a mistake with sqlalchemy though, since it's revealed as Any as you mentioned. pandas\core\generic.py:2619: error: Revealed type is 'Union[Any, sqlite3.dbapi2.Connection]' pandas\core\generic.py:2620: error: Revealed type is 'Union[builtins.str, def (pandas.io.sql.SQLTable, Union[Any, sqlite3.dbapi2.Connection], builtins.list[builtins.str], typing.Iterator[builtins.list[Any]]), None]' I'll figure sqlalchemy types out and make sure this passes the tests out. Also, here is the cross-module example: https://mypy.readthedocs.io/en/latest/common_issues.html#import-cycles — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub, or mute the thread.

oguzhanogreden · 2019-06-17T19:40:58Z

Of course, should've thought! :) This has been very educative for me, thanks.

Once you folks decide how to treat these annotations, I'll move on as you suggest.

pandas/core/generic.py

oguzhanogreden · 2019-07-31T06:34:33Z

I want to move this forward.

We have two issues:

SQLTable can't be imported due to circular import,
sqlalchemy.engine.Engine can't be imported due to it being an optional dependency.

Quick recap of what happened so far: Initially we added Any for these and left comments. Then I was pointed to quoting these with backticks, which led me down to TYPE_CHECKING conditional. It turned out that's something you don't want.

I will go back to "Any and comments" solution for SQLTable and sqlalchemy.engine.Engine. I think this is also what's implied by @WillAyd's last request changes. Is this acceptable by the group at this stage? If there is not a consensus, I'd like to remove type annotations from this PR and focus on the initial goal.

Let me know!

WillAyd · 2019-08-26T16:22:13Z

I would say go ahead and remove the complicated annotations that are tripping this up and we can get this in

pandas/core/generic.py

oguzhanogreden · 2019-08-29T18:07:34Z

This should be ready.

TomAugspurger · 2019-08-30T17:09:22Z

Thanks! Congrats on your first PR to pandas!

WillAyd · 2019-08-30T17:10:42Z

Yea nice job @oguzhanogreden thanks for sticking through the back and forth.

oguzhanogreden · 2019-08-31T19:15:26Z

Great, thanks for your support and patience as well!

…das-dev#11886 (pandas-dev#26795) * DOC: add single dtype to NDFrame.to_sql

oguzhanogreden added 2 commits June 11, 2019 22:22

DOC: add single dtype to NDFrame.to_sql

12591d1

Now passes flake8

4397fc7

WillAyd requested changes Jun 11, 2019

View reviewed changes

pandas/core/generic.py Outdated Show resolved Hide resolved

pandas/core/generic.py Outdated Show resolved Hide resolved

WillAyd added the Docs label Jun 11, 2019

oguzhanogreden and others added 4 commits June 13, 2019 21:57

Update pandas/core/generic.py

e9b09c6

Co-Authored-By: William Ayd <[email protected]>

Update pandas/core/generic.py

d7990f2

Co-Authored-By: William Ayd <[email protected]>

Type annotations for pandas.core.generic.to_sql()

47bafad

Aligned documentation for pandas.io.sql.to_sql()

53a364f

Now passes flake8

d535914

oguzhanogreden commented Jun 13, 2019

View reviewed changes

pandas/core/generic.py Outdated Show resolved Hide resolved

oguzhanogreden commented Jun 13, 2019

View reviewed changes

pandas/io/sql.py Show resolved Hide resolved

jorisvandenbossche reviewed Jun 13, 2019

View reviewed changes

pandas/io/sql.py Outdated Show resolved Hide resolved

WillAyd requested changes Jun 13, 2019

View reviewed changes

WillAyd requested changes Jun 14, 2019

View reviewed changes

pandas/core/generic.py Outdated Show resolved Hide resolved

oguzhanogreden added 2 commits June 15, 2019 14:11

Completed type annotations for generic.NDFrame.to_sql()

680ed87

Follow docstring guide for parameter types of pandas.io.sql

ac44371

oguzhanogreden mentioned this pull request Jun 15, 2019

DOC: Reference to type 'pd_table' in IO Tools User Guide #26868

Closed

WillAyd added the Typing type annotations, mypy/pyright type checking label Jun 15, 2019

Avoid circular import

52a6782

WillAyd requested changes Jun 15, 2019

View reviewed changes

Use only _typing.Dtype for dtype=

13633be

WillAyd approved these changes Jun 16, 2019

View reviewed changes

jreback requested changes Jun 16, 2019

View reviewed changes

pandas/core/generic.py Outdated Show resolved Hide resolved

oguzhanogreden added 2 commits June 17, 2019 07:59

Add type checking only imports

82ebc5c

Merge branch 'master' into docs-pr11886

4700af9

oguzhanogreden commented Jun 17, 2019

View reviewed changes

pandas/core/generic.py Outdated Show resolved Hide resolved

WillAyd requested changes Jun 22, 2019

View reviewed changes

pandas/core/generic.py Outdated Show resolved Hide resolved

pandas/core/generic.py Outdated Show resolved Hide resolved

simonjayhawkins mentioned this pull request Jul 22, 2019

Removed ABCs from pandas._typing #27424

Merged

5 tasks

oguzhanogreden added 2 commits August 29, 2019 11:38

merge upstream/master

9eee1cf

Merge branch 'master' into docs-pr11886

63ed2e3

oguzhanogreden commented Aug 29, 2019

View reviewed changes

pandas/core/generic.py Outdated Show resolved Hide resolved

oguzhanogreden added 3 commits August 29, 2019 11:56

Clean up merge

b03e22f

Remove whitespace

00a081b

Fix typo

efa97be

TomAugspurger added this to the 1.0 milestone Aug 30, 2019

TomAugspurger approved these changes Aug 30, 2019

View reviewed changes

TomAugspurger merged commit 621ad9d into pandas-dev:master Aug 30, 2019

oguzhanogreden deleted the docs-pr11886 branch August 31, 2019 19:15

MarcoGorelli pushed a commit to MarcoGorelli/pandas that referenced this pull request Sep 3, 2019

DOC: Document existing functionality of pandas.DataFrame.to_sql() pan…

9c5eb32

…das-dev#11886 (pandas-dev#26795) * DOC: add single dtype to NDFrame.to_sql

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

DOC: Document existing functionality of pandas.DataFrame.to_sql() pan…

e0bb3ad

…das-dev#11886 (pandas-dev#26795) * DOC: add single dtype to NDFrame.to_sql

proost pushed a commit to proost/pandas that referenced this pull request Dec 19, 2019

DOC: Document existing functionality of pandas.DataFrame.to_sql() pan…

9da3723

…das-dev#11886 (pandas-dev#26795) * DOC: add single dtype to NDFrame.to_sql

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

DOC: Document existing functionality of pandas.DataFrame.to_sql() #11886 #26795

DOC: Document existing functionality of pandas.DataFrame.to_sql() #11886 #26795

oguzhanogreden commented Jun 11, 2019

oguzhanogreden commented Jun 11, 2019

WillAyd left a comment

codecov bot commented Jun 11, 2019 •

edited

Loading

oguzhanogreden commented Jun 12, 2019

WillAyd commented Jun 12, 2019

pep8speaks commented Jun 13, 2019 •

edited

Loading

oguzhanogreden Jun 13, 2019

WillAyd Jun 13, 2019

WillAyd left a comment

WillAyd Jun 13, 2019

WillAyd Jun 13, 2019

WillAyd left a comment

oguzhanogreden commented Jun 15, 2019

WillAyd commented Jun 15, 2019

WillAyd commented Jun 15, 2019

oguzhanogreden commented Jun 15, 2019

WillAyd Jun 15, 2019

oguzhanogreden Jun 15, 2019

WillAyd Jun 15, 2019

WillAyd left a comment

oguzhanogreden commented Jun 17, 2019

WillAyd commented Jun 17, 2019 via email

oguzhanogreden commented Jun 17, 2019

oguzhanogreden commented Jul 31, 2019 •

edited

Loading

WillAyd commented Aug 26, 2019

oguzhanogreden commented Aug 29, 2019

TomAugspurger commented Aug 30, 2019

WillAyd commented Aug 30, 2019

oguzhanogreden commented Aug 31, 2019

DOC: Document existing functionality of pandas.DataFrame.to_sql() #11886 #26795

DOC: Document existing functionality of pandas.DataFrame.to_sql() #11886 #26795

Conversation

oguzhanogreden commented Jun 11, 2019

oguzhanogreden commented Jun 11, 2019

WillAyd left a comment

Choose a reason for hiding this comment

codecov bot commented Jun 11, 2019 • edited Loading

Codecov Report

oguzhanogreden commented Jun 12, 2019

WillAyd commented Jun 12, 2019

pep8speaks commented Jun 13, 2019 • edited Loading

Comment last updated at 2019-08-29 10:01:59 UTC

oguzhanogreden Jun 13, 2019

Choose a reason for hiding this comment

WillAyd Jun 13, 2019

Choose a reason for hiding this comment

WillAyd left a comment

Choose a reason for hiding this comment

WillAyd Jun 13, 2019

Choose a reason for hiding this comment

WillAyd Jun 13, 2019

Choose a reason for hiding this comment

WillAyd left a comment

Choose a reason for hiding this comment

oguzhanogreden commented Jun 15, 2019

WillAyd commented Jun 15, 2019

WillAyd commented Jun 15, 2019

oguzhanogreden commented Jun 15, 2019

WillAyd Jun 15, 2019

Choose a reason for hiding this comment

oguzhanogreden Jun 15, 2019

Choose a reason for hiding this comment

WillAyd Jun 15, 2019

Choose a reason for hiding this comment

WillAyd left a comment

Choose a reason for hiding this comment

oguzhanogreden commented Jun 17, 2019

WillAyd commented Jun 17, 2019 via email

oguzhanogreden commented Jun 17, 2019

oguzhanogreden commented Jul 31, 2019 • edited Loading

WillAyd commented Aug 26, 2019

oguzhanogreden commented Aug 29, 2019

TomAugspurger commented Aug 30, 2019

WillAyd commented Aug 30, 2019

oguzhanogreden commented Aug 31, 2019

codecov bot commented Jun 11, 2019 •

edited

Loading

pep8speaks commented Jun 13, 2019 •

edited

Loading

oguzhanogreden commented Jul 31, 2019 •

edited

Loading